src/ai/hybridSearch.ts (1)

55-60: ⚠️ Potential issue | 🟠 Major

Index both engines from the same chunk array.

SemanticSearchService.indexNote() already re-chunks internally (src/ai/semanticSearch.ts:42-62), and this method chunks the raw content again for BM25. If those chunkers ever diverge, the same chunk ID can point to different text in each engine and RRF will fuse unrelated hits. Generate the chunks once here and feed that exact array to both indexes.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ai/hybridSearch.ts` around lines 55 - 60, The code currently chunks
content twice causing mismatched chunk IDs; call this.chunkFn(noteId, content)
once to produce a single chunks array and feed that same array to both engines.
Update the call site in hybridSearch.indexNote to use the single chunks array
and change SemanticSearchService.indexNote (or add a new
semanticSearch.indexChunks) to accept the precomputed chunks array instead of
raw content so it does not re-chunk internally; then pass chunks to
keywordEngine.indexChunks(chunks) and to the semantic search method, and remove
the internal re-chunking in SemanticSearchService.

src/ai/hybridSearch.ts (2)

72-83: ⚠️ Potential issue | 🟠 Major

Filter keyword hits before truncating to topK.

KeywordSearchEngine.search() slices before returning, so filtering that reduced list at Line 81 can drop valid note-scoped matches whenever other notes occupy the first broadK BM25 slots.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ai/hybridSearch.ts` around lines 72 - 83, Keyword search results are
pre-sliced by KeywordSearchEngine.search, so filtering by opts.noteId after that
(into filteredKeyword) can drop valid note-scoped hits; change the flow to fetch
untruncated keyword hits (e.g., expose/use a searchAll / unsliced variant or
call keywordEngine.search with a sufficiently large K) then apply the noteId
filter (opts.noteId) to that full set and only after filtering truncate to
broadK (used alongside semanticResults); update usages around
keywordEngine.search, filteredKeyword, broadK, and opts.noteId accordingly.

---

47-49: ⚠️ Potential issue | 🟠 Major

Persist or rebuild the BM25 index on startup.

initialize() restores only the semantic store, and save() persists only that same state. After a restart, keywordEngine is empty, so hybrid search silently degrades to semantic-only until every note is reindexed.

Also applies to: 96-98

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ai/hybridSearch.ts` around lines 47 - 49, initialize() currently only
restores the semantic store so keywordEngine/BM25 is empty after restart; update
initialize() (and the save() path referenced near lines 96-98) to persist and
restore the BM25/keywordEngine state or trigger a rebuild: add serialization of
the BM25 index when save() runs and load/deserializer logic in initialize() to
repopulate keywordEngine, and if no serialized snapshot exists call the existing
reindex routine (e.g., reindexAll or the method that iterates notes) to rebuild
the BM25 index on startup; ensure you reference and update keywordEngine,
initialize(), save(), and the BM25 serialization/rebuild helpers so hybridSearch
is fully restored after restart.

src/ai/vectorStore.ts (1)

149-165: ⚠️ Potential issue | 🟠 Major

Validate every loaded row before rebuilding the map.

This path only proves that the JSON parses and that the first entry matches vectorDim. Valid JSON with the wrong shape can still fail with opaque TypeErrors at Line 149 or Line 161, and a later mixed-dimension row slips through until search time. Validate entries up front and check each row's chunk fields plus vector length before populating this.entries.

Proposed fix

+        if (!Array.isArray(data.entries)) {
+            throw new Error(
+                `VectorStore: invalid index structure in "${filePath}". ` +
+                `Delete the file to rebuild the index.`,
+            );
+        }
+
-        if (data.vectorDim !== undefined && data.entries.length > 0) {
-            const actualDim = data.entries[0].vector.length;
-            if (actualDim !== data.vectorDim) {
-                throw new Error(
-                    `VectorStore: index vectorDim mismatch ` +
-                    `(stored ${data.vectorDim}, found ${actualDim}). Index may be corrupted.`,
-                );
-            }
-        }
-
         this.entries.clear();
-        for (const entry of data.entries) {
+        for (const [index, entry] of data.entries.entries()) {
+            if (
+                !entry ||
+                !entry.chunk ||
+                typeof entry.chunk.id !== "string" ||
+                typeof entry.chunk.noteId !== "string" ||
+                typeof entry.chunk.content !== "string" ||
+                typeof entry.chunk.chunkIndex !== "number" ||
+                !Array.isArray(entry.vector)
+            ) {
+                throw new Error(
+                    `VectorStore: invalid entry at index ${index} in "${filePath}". ` +
+                    `Delete the file to rebuild the index.`,
+                );
+            }
+            if (
+                data.vectorDim !== undefined &&
+                entry.vector.length !== data.vectorDim
+            ) {
+                throw new Error(
+                    `VectorStore: invalid vector dimension at entry ${index} in "${filePath}".`,
+                );
+            }
             this.entries.set(entry.chunk.id, {
                 chunk: entry.chunk,
                 vector: entry.vector,
             });
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ai/vectorStore.ts` around lines 149 - 165, Validate the loaded payload
before rebuilding this.entries: confirm data.entries is an array and iterate
each entry to ensure it has a chunk object with required fields (at least
chunk.id) and a numeric vector array whose length matches data.vectorDim (when
data.vectorDim is defined) and all elements are numbers; throw a clear Error
identifying the offending entry if any check fails; only after all entries pass
validation, clear this.entries and populate it using entry.chunk.id -> { chunk:
entry.chunk, vector: entry.vector } so malformed rows never get inserted
(referencing data.entries, data.vectorDim, entry.chunk, entry.vector, and
this.entries).

src/ai/__tests__/vectorStore.test.ts (1)

188-199: ⚠️ Potential issue | 🟡 Minor

Assert the no-op, not just the final contents.

Line 191 says the second save() should be a no-op, but this test still passes if save() rewrites the same JSON. Capture file metadata before/after the second call, or spy on the write path, so the test proves the optimization instead of only re-reading the final file contents.

Proposed fix

-import { mkdtemp, rm, readFile, writeFile } from "node:fs/promises";
+import { mkdtemp, rm, readFile, writeFile, stat } from "node:fs/promises";
...
         it("skips write when nothing changed", async () => {
             store.add([makeChunk("n1", 0, [1, 0, 0])]);
             await store.save();
+            const before = await stat(join(tempDir, "test-index.json"));
             await store.save(); // second save should be no-op
+            const after = await stat(join(tempDir, "test-index.json"));

             const content = await readFile(
                 join(tempDir, "test-index.json"),
                 "utf-8",
             );
             const data = JSON.parse(content);
+            expect(after.mtimeMs).toBe(before.mtimeMs);
             expect(data.version).toBe(1);
             expect(data.entries).toHaveLength(1);
         });

As per coding guidelines, **/*.test.{ts,tsx,js,jsx}: The tests are not tautological.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ai/__tests__/vectorStore.test.ts` around lines 188 - 199, The test
currently only verifies final file contents but must assert the second
store.save() is a true no-op; modify the "skips write when nothing changed" test
to capture file metadata or spy the write path before the second save (e.g.,
stat the file mtime or spy on fs.writeFile/Store's internal write method used by
save()), call await store.save() the second time, then assert that no write
occurred (mtime unchanged or spy not called) in addition to the existing content
checks; locate the logic around store.save() and the file read (readFile/join
usage) to add the stat/spy and the assertion.

src/ai/vectorStore.ts (1)

164-179: ⚠️ Potential issue | 🟠 Major

Validate every loaded entry before rebuilding the index.

At Line 164-Line 179, load() still trusts persisted JSON shape and only checks dimension consistency using the first vector. A malformed/corrupted file can still inject invalid entries or mixed dimensions, causing downstream runtime failures during search.

🔧 Proposed hardening for load-time validation

-        if (data.vectorDim !== undefined && data.entries.length > 0) {
-            const actualDim = data.entries[0].vector.length;
-            if (actualDim !== data.vectorDim) {
-                throw new Error(
-                    `VectorStore: index vectorDim mismatch ` +
-                    `(stored ${data.vectorDim}, found ${actualDim}). Index may be corrupted.`,
-                );
-            }
-        }
-
-        this.entries.clear();
-        for (const entry of data.entries) {
-            this.entries.set(entry.chunk.id, {
-                chunk: entry.chunk,
-                vector: entry.vector,
-            });
-        }
+        if (!data || !Array.isArray(data.entries)) {
+            throw new Error(
+                `VectorStore: invalid index structure in "${filePath}". Delete it to rebuild.`,
+            );
+        }
+
+        const expectedDim =
+            data.vectorDim ??
+            (data.entries.length > 0 ? data.entries[0].vector.length : undefined);
+
+        this.entries.clear();
+        for (let i = 0; i < data.entries.length; i++) {
+            const entry = data.entries[i];
+            if (
+                !entry ||
+                !entry.chunk ||
+                typeof entry.chunk.id !== "string" ||
+                typeof entry.chunk.noteId !== "string" ||
+                !Array.isArray(entry.vector)
+            ) {
+                throw new Error(
+                    `VectorStore: invalid entry at index ${i} in "${filePath}". Delete it to rebuild.`,
+                );
+            }
+
+            if (expectedDim !== undefined && entry.vector.length !== expectedDim) {
+                throw new Error(
+                    `VectorStore: inconsistent vector dimensions in "${filePath}" ` +
+                    `(expected ${expectedDim}, found ${entry.vector.length} at entry ${i}).`,
+                );
+            }
+
+            this.entries.set(entry.chunk.id, {
+                chunk: entry.chunk,
+                vector: entry.vector,
+            });
+        }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ai/vectorStore.ts` around lines 164 - 179, The load() path currently only
checks vectorDim against the first entry and then trusts data.entries; instead,
validate every entry before rebuilding this.entries: iterate over data.entries
and for each entry confirm entry.chunk and entry.chunk.id exist, entry.vector is
an array of numbers, and entry.vector.length matches data.vectorDim (or infer
and set vectorDim if undefined) and throw a descriptive Error on mismatch; only
after all entries pass validation populate this.entries (used in the for loop
that calls this.entries.set) to avoid injecting malformed or mixed-dimension
vectors into the index.

src/ai/__tests__/vectorStore.test.ts (1)

188-200: ⚠️ Potential issue | 🟡 Minor

Make the no-op save test prove no second write happened.

Line 188-Line 200 currently validates content only, so it still passes if save() rewrites identical JSON on the second call.

✅ Proposed test tightening

-import { mkdtemp, rm, readFile, writeFile } from "node:fs/promises";
+import { mkdtemp, rm, readFile, writeFile, stat } from "node:fs/promises";
@@
         it("skips write when nothing changed", async () => {
             store.add([makeChunk("n1", 0, [1, 0, 0])]);
             await store.save();
+            const before = await stat(join(tempDir, "test-index.json"));
             await store.save(); // second save should be no-op
+            const after = await stat(join(tempDir, "test-index.json"));
 
             const content = await readFile(
                 join(tempDir, "test-index.json"),
                 "utf-8",
             );
             const data = JSON.parse(content);
+            expect(after.mtimeMs).toBe(before.mtimeMs);
             expect(data.version).toBe(1);
             expect(data.entries).toHaveLength(1);
         });

As per coding guidelines, **/*.test.{ts,tsx,js,jsx}: The tests are not tautological.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ai/__tests__/vectorStore.test.ts` around lines 188 - 200, The test "skips
write when nothing changed" currently only asserts file contents, so it can pass
if save() rewrites identical JSON; update the test to prove the second save did
not perform a write by either (a) spying on the file write API used by
VectorStore (e.g. jest.spyOn(fs.promises, "writeFile") or the actual writer used
by store.save) and asserting it was called exactly once, or (b) recording the
file mtime via fs.promises.stat on "test-index.json" before the second await
store.save() and asserting the mtime did not change after the second save;
reference the test name, store.save, and the "test-index.json" file when adding
the spy/stat checks.